#161: make three VICE suites measure what they claim (mutation-proven) - #176

Closes #161.

Three VICE suites passed or failed for reasons unrelated to what they claim to
measure. All three share one shape: the rig never establishes a precondition
the routine under test relies on, and whether that precondition is needed is
build-configuration dependent — so the defect is invisible on the backend the
suite was written against.

Per the issue thread's own prescription, nothing here is argued from reading
code. Each claim was proved by mutating the code under test and showing the
suite did not notice, then re-running the same mutation after the fix and
showing it goes red.

Method

Everything below is BACKEND=uci, C64_SKIP_BUILD=1, VICE via
c64-test-harness with -reu -reusize 512
(tools/_vice_helpers.py::default_vice_config).
Unmutated PRG sha256 7c79a4e00a19685de7540970247c759a5e1cc7e13a88b12bf8b8234fcc013c14;
every mutant was rebuilt and its PRG hash confirmed different before running,
and confirmed back to 7c79a4e0… after restoring.

Per-suite table

Note the third row's post-fix 12/12: deframer/wikipedia_shaped_flight had been
red before any of this work, and it was red for the empty-hostname reason.

Verbatim mutation-kill evidence

Suite 1 — test_finished_verify.py, mutation M-C

        eor tls_verify_data,x
        ora zp_tmp1                     ; accumulate differences
        sta zp_tmp1
        iny
        inx
        cpx #1                          ; MUTATION M-C: compare only byte 0
        bne @vf_cmp

PRG 6c929c504f8f16082769bea9deed4512ea05ee34f4c5100ec6fd0f42a353d5c0.

Pre-fix, mutated — indistinguishable from the unmutated baseline:

--- Vector set A (RFC 8448 s_hs_traffic) ---
  FAIL: positive          want C=0 accept, got C=1 reject
  PASS: flip_first_byte   want C=1 reject, got C=1 reject
  PASS: flip_last_byte    want C=1 reject, got C=1 reject
  PASS: all_zeros         want C=1 reject, got C=1 reject
  PASS: all_ones          want C=1 reject, got C=1 reject
  PASS: truncated         want C=1 reject, got C=1 reject
  PASS: rotated           want C=1 reject, got C=1 reject
  PASS: wrong_secret      want C=1 reject, got C=1 reject
  PASS: wrong_transcript  want C=1 reject, got C=1 reject
...
  Passed: 16/18
  Failed: 2/18

Post-fix, same mutation — killed:

--- Vector set A (RFC 8448 s_hs_traffic) ---
  PASS: positive          want C=0 accept, got C=0 accept
  PASS: flip_first_byte   want C=1 reject, got C=1 reject
  FAIL: flip_last_byte    want C=1 reject, got C=0 accept
  PASS: all_zeros         want C=1 reject, got C=1 reject
  PASS: all_ones          want C=1 reject, got C=1 reject
  FAIL: truncated         want C=1 reject, got C=0 accept
  PASS: rotated           want C=1 reject, got C=1 reject
  PASS: wrong_secret      want C=1 reject, got C=1 reject
  PASS: wrong_transcript  want C=1 reject, got C=1 reject
...
  Passed: 14/18
  Failed: 4/18

Mutation restored (PRG back to 7c79a4e0…):

  Passed: 18/18
  Failed: 0/18

  [+] Finished verify: ALL 18 TESTS PASSED

Suite 2 — test_tls_deframe.py, mutation M-B

@ctx:
        ; certificate_request_context length — must be 0 for a server
        ; Certificate (RFC 8446 §4.4.2)
        jsr df_cs_take_byte
        ; MUTATION M-B: certificate_request_context zero-check DELETED
:       lda #CS_LIST_LEN

PRG e99a92782ac78640187e446ac923eb42a3a7008e331b9d95d76ea2a4bb4698b5.

Pre-fix, mutated — diff against the unmutated baseline transcript shows
only the two freshly-minted-cert hash values; the verdict set is identical:

35c35
<   FAIL  streamed Certificate chain (transcript)  got a6b3f3b2… want 06625365…
---
>   FAIL  streamed Certificate chain (transcript)  got 69c79844… want 65b47a20…
40c40
<   FAIL  streamed Certificate + trailing message (transcript)  got 97910dee… want b4cdbfd0…
---
>   FAIL  streamed Certificate + trailing message (transcript)  got 59cbc6e1… want 9ae184c9…

and the case whose guard was deleted:

  PASS  non-zero request context rejected (streamed)
...
  Passed: 40/45
  Failed: 5/45

Post-fix, same mutation — killed:

  PASS  in-place Certificate
  PASS  streamed Certificate chain
  PASS  streamed Certificate + trailing message
  PASS  2048 B leaf passes the cap (fails later on extraction)
  PASS  1636 B (wikipedia) leaf passes the cap
  FAIL  non-zero request context rejected (streamed)  events=['msg']
  PASS  junk leaf rejected (no usable key)

  Passed: 44/45
  Failed: 1/45

Mutation restored:

  Passed: 45/45
  Failed: 0/45

  [+] TLS deframe: ALL 45 TESTS PASSED

Suite 3 — test_tls_deframer.py, mutation M-D

        lda df_cs_entry
        bne @cert_fmt_err       ; MUTATION M-D: inverted (complete Certificate errors)
        lda #$34

PRG b8698374488961efa12a915aac9fb612eda2ce580e40bea217e44498751a83a0.

Pre-fix, mutated — identical to the unmutated baseline:

  PASS [deframer]  deframer/cert_split_mid_body
  PASS [deframer]  deframer/cert_many_records
  PASS [deframer]  deframer/mfl512_full_records
  FAIL [deframer]  deframer/wikipedia_shaped_flight
    transcript mismatch (a message's bytes were not folded)
  PASS [deframer]  deframer/oversized_leaf_errors
  PASS [deframer]  deframer/wikipedia_sized_leaf_accepted

  plumbing (must pass on ANY build): 2/2 passed
  deframer (W1/W2 acceptance gate):  11/12 passed

Post-fix, same mutation — killed:

  tls_hostname = 'www.foo.invalid' (from the fixture cert's SAN)
  PASS [deframer]  deframer/two_msgs_one_record
  FAIL [deframer]  deframer/cert_split_mid_body
    flight ABORTED (tls_recv_encrypted returned C=1 after 0 successful call(s)) — the client refused this Certificate; carries=[1]
  FAIL [deframer]  deframer/cert_split_in_header@1
    flight ABORTED (tls_recv_encrypted returned C=1 after 0 successful call(s)) — the client refused this Certificate; carries=[1]
  ...
  FAIL [deframer]  deframer/wikipedia_shaped_flight
    flight ABORTED (tls_recv_encrypted returned C=1 after 1 successful call(s)) — the client refused this Certificate; carries=[0, 1]
  PASS [deframer]  deframer/oversized_leaf_errors
  FAIL [deframer]  deframer/wikipedia_sized_leaf_accepted
    flight ABORTED (tls_recv_encrypted returned C=1 after 0 successful call(s)) — the client refused this Certificate; carries=[1]

  plumbing (must pass on ANY build): 2/2 passed
  deframer (W1/W2 acceptance gate):  3/12 passed

Mutation restored:

  plumbing (must pass on ANY build): 2/2 passed
  deframer (W1/W2 acceptance gate):  12/12 passed

  [+] Deframer acceptance gate: ALL scenarios PASSED

Profile split: the ip65 control (added after review)

Suite 1's defect is a pure profile split, and both halves of that are now
measured rather than argued. ip65 built in this worktree; blob
ip65-build/ip65-c64.bin verified cf1a5ff7809af4e4655e385b378b936054f41046ff2b7604828af3240c2d90dd,
6,951 B — the worktree reads its own blob (#116).

The fifth row is the control: on ip65 the pre-fix rig was already
discriminating, because tls_verify_finished calls tls_hs_ptr_reset itself
there. So the suite was sound on the profile tools/run_all_tests.py builds
and vacuous on the profile that ships and reaches real servers. One cause,
two symptoms — the positives failing and the negatives passing are the same
stale pointer seen from opposite sides — and the honest-accept control did not
hold on the backend that matters.

The last row also measures the "no-op on ip65" claim instead of asserting it:
the added jsr tls_hs_ptr_reset leaves ip65 at 18/18.

What changed

• tools/test_finished_verify.py — the carry stub is now
  jsr tls_hs_ptr_reset; jsr tls_verify_finished; … (13 B; the carry latch
  moved $034C → $0350 to stay clear of it). Calling the repo's own entry point
  rather than poking $3E/$3F from Python keeps one copy of the base address.
• src/tls_keyschedule.s, src/tls_cert.s — the docstrings that promised
  direct callers the pointer "is reset at entry" (and named
  test_finished_verify.py as the beneficiary) now say who actually sets it
  under which build. This is issue item 2, and it is what hid the failure.
  Comment-only: PRG sha256 is unchanged at 7c79a4e0….
• tools/test_tls_deframe.py — generate_p256_cert() mints with a SAN;
  install_hostname() writes tls_hostname/tls_hostname_len. Both take the
  name from one module constant CERT_HOST (deframe.foo.invalid, reserved
  TLD per tools/test_reserved_test_host.py), so cert identity and requested
  identity cannot drift apart into a DF_ERR_CERT_FMT nobody can attribute.
• tools/test_tls_deframer.py — load_cert_fixture() reads the SAN dNSNames
  back out of the emitted DER and they become tls_hostname (derived, never
  spelled); build_wiki_leaf() mints its leaf with the same SAN; and
  run_scenario() now judges the flight outcome, not just its side effects.
• tools/https_e2e/chain_certs.py — build_padded_intermediate(…, sans=None).
  Default is None, so ensure_chain_certs' padding intermediates are
  byte-unchanged (verified: still exactly one extension, 1.3.6.1.4.1.55555.1).

Is production affected? No — and here is the trace, not the assertion

Raised in review, because "we changed the test until it passed" is what a
comment-only source diff looks like without this section.

1. In a real uci handshake, is tls_hs_ptr always set before the verify?

Yes, and the reachable set is small enough to enumerate.

tls_verify_finished has exactly two call sites in the tree:

So on a uci build the deframer's df_dispatch is the only production caller.
df_dispatch is not exported (src/tls_deframe.s:76-91 exports
tls_deframe_{init,new_record,pump}, df_* state and tls_hs_allowed — not
the dispatcher), and it has exactly two entries, jmp df_dispatch at lines 334
and 425. Both write tls_hs_ptr in the same basic block, a few instructions
earlier, with no branch in between:

• in-place route — src/tls_deframe.s:311-334: jsr df_rec_cursor gives
  the body start, sbc #4 backs up to the message header, stored to
  tls_hs_ptr/+1 at 316-319; then transcript fold, cursor advance,
  jmp df_dispatch. Straight-line.
• carry route — src/tls_deframe.s:418-425: on message completion,
  tls_hs_ptr = df_carry_buf at 420-424, then jmp df_dispatch. Straight-line.

The third route, df_stream_begin, handles TLS_HS_CERTIFICATE only
(:338-341) and never reaches df_dispatch at all (:759 — it consumes the
message incrementally). A Finished cannot take it.

There is no route to the verify that does not write the pointer immediately
beforehand. It is never stale in production.

2. If it were stale, which way does it fail?

Fail-closed. Confirmed — your reading is right. Two independent reasons:

• The expected value is computed from tls_s_hs_secret and tls_transcript,
  neither of which tls_hs_ptr influences. A stale pointer only changes which
  32 bytes are compared against it. For acceptance the attacker must place
  the correct verify_data at the stale address — but computing it requires
  the server handshake traffic secret, and anyone holding that can simply send
  a correct Finished and be authenticated legitimately. No advantage.

• The one theoretical accept-anything shape is a pointer aimed at the expected
  value itself — if (tls_hs_ptr)+4 landed on tls_verify_data, the routine
  would compare the buffer with itself and always match. It is out of range:
  tls_verify_data = $B309, while the pointer's entire reachable set is
  tls_rec_buf = $A000 .. $A223 (548 B, src/data.s:276) for the in-place
  route and the fixed df_carry_buf = $3B89 for the carry route. $B309 is
  4,326 B above the top of the in-place window.

  Flagging that second one honestly: it is closed by layout, and the issue
  thread's own canonical case is a guard that held by layout rather than logic.
  It is moot only because (1) holds — the pointer is never stale — but if the
  guard in (1) were ever weakened, this is the property that would need a real
  pin (a link-time .assert that the two ranges are disjoint). Not added here:
  out of scope for Three VICE suites pass or fail for reasons unrelated to what they measure (test_finished_verify's 16 negatives are vacuous on UCI) #161, and it would be pinning a counterfactual.

So: not a security fix. No path accepts a forged Finished.

3. Should the .ifndef guard get a functional fix?

No — the guard is correct, and removing it is a regression. Measured, not
argued. Mutation M-E: delete the .ifndef TLS_STREAM_DEFRAME wrapper so
the reset runs unconditionally, uci PRG
e0dffcaa885502d01444ac0d6721fa40917b4b35e8e0fb432b26e9fb69bd0b6a:

  FAIL  split Finished accepted (per-message transcript)  events=['msg', ('err', 3)]
  PASS  split Finished accepted (transcript)
  FAIL  EE+Finished sharing one record  events=['msg', ('err', 3)]
  PASS  split Finished REJECTED on bad verify_data

err 3 is DF_ERR_DISPATCH — a valid server Finished rejected, handshake
aborted. Both broken cases are real wire shapes the deframer exists for: a
Finished that arrived split across records (pointer = df_carry_buf), and a
Finished sharing a record with EncryptedExtensions (pointer =
tls_rec_buf + offset). Resetting to tls_rec_buf in either case reads the
wrong 32 bytes. Restored → 45/45, PRG back to 7c79a4e0….

Note the third line of that output: split Finished REJECTED on bad verify_data still passes under M-E — it rejects for the wrong reason, the same
shape this PR is about. Even the regression is partly invisible to the suite.

The defect was therefore in the documentation and the harness, which is
what this PR changes. Leaving src/tls_keyschedule.s functionally unchanged
(PRG sha256 identical) is the correct call, and the .ifndef should stay.

On the issue thread's "pin rather than restate" rule

Where a fact had to appear twice, it is now derived rather than copied:
the rig calls tls_hs_ptr_reset instead of hardcoding $3E; suite 3 reads the
hostname out of the certificate it is about to send; suite 2 uses one constant
for both the SAN and tls_hostname. The thread's second qualification applies
to this PR as much as to any other: mutation testing is a floor. M-C, M-B
and M-D are three mutants somebody thought of. They do not prove these suites
now catch the fourth.

Not done / caveats

• ip65 WAS built and run for suite 1 only (see the profile-split table).
  Suites 2 and 3 are uci-only by construction (TLS_STREAM_DEFRAME), so no
  ip65 result exists for them and none is claimed.
• No hardware. These are VICE suites; the U64E/C64U were not touched.
• tools/test_x509.py drives x509_parse_cert over DMA and does not set
  tls_hostname. It builds ip65 by default, where the name check is compiled
  out, so it is not affected today — but it is the shape the issue warns about,
  and it is not covered here. tools/test_tls_p384_negotiation.py and
  tools/test_hs_sequence.py were checked and already establish their
  preconditions (tls_hs_ptr_reset and tls_hostname respectively).
• Suite 2's fix does not add a negative name case (a cert whose SAN does not
  match tls_hostname), so it cannot detect an x509_verify_hostname that
  accepts everything. tools/test_x509_name.py owns that; keeping this PR to
  the reported defect.
• pytest at repo root: 50 passed. tools/test_reserved_test_host.py: 9/9.